Skip to content

Install built doc to SAGE_DOC #40807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
kwankyu wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Install built doc to SAGE_DOC #40807

kwankyu wants to merge 5 commits into from

Conversation

@kwankyu
Copy link
Collaborator

@kwankyu kwankyu commented Sep 12, 2025 •
edited
Loading

This is a solution for https://groups.google.com/g/sage-devel/c/IVo1EI33CyU, alternative to #40798.

With this PR

make doc-html

installs the built doc to the expected location (set by SAGE_DOC). The same with

make doc-pdf

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@kwankyu kwankyu marked this pull request as ready for review September 12, 2025 04:22
@kwankyu kwankyu mentioned this pull request Sep 12, 2025
Closed
5 tasks
@github-actions
Copy link

github-actions bot commented Sep 12, 2025 •
edited
Loading

Documentation preview for this PR (built with commit f7cafd3; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@antonio-rojas
Copy link
Contributor

antonio-rojas commented Sep 13, 2025

IMO it would be better to do this at the meson level (with install-doc-{html,pdf} targets) and then Makefile would just call these targets. That way, other distributions could also make use of it instead of manually copying docs as it is done now.

That being said, this change only affects sage-the-distro so I have no objection as a temporary stopgap.

tobiasdiez
tobiasdiez reviewed Sep 14, 2025
View reviewed changes
src/doc/Makefile Outdated
doc-html:
meson compile -C ../../build/sage-distro doc-html
mkdir -p $(SAGE_DOC)
cp -rf ../../build/sage-distro/src/doc/html $(SAGE_DOC)
Copy link
Contributor

@tobiasdiez tobiasdiez Sep 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does SAGE_DOC points to on your system, i.e. where do those docs end up in?

Copy link
Collaborator Author

@kwankyu kwankyu Sep 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sage-the-distro, it is local/share/doc at sage root by default. The Makefile runs after environment variables for sage-the-distro are setup.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 14, 2025

IMO it would be better to do this at the meson level (with install-doc-{html,pdf} targets) and then Makefile would just call these targets. That way, other distributions could also make use of it instead of manually copying docs as it is done now.

Yes. Then src/doc/meson.build should be modified.

That being said, this change only affects sage-the-distro so I have no objection as a temporary stopgap.

Yes.

@kwankyu kwankyu marked this pull request as draft September 15, 2025 22:47
@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 15, 2025

Strangely, doctrees directory is also copied, in my testing. Let me fix this.

@kwankyu kwankyu marked this pull request as ready for review September 16, 2025 02:28
@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 16, 2025

There must have been some confusion on my side. I tested again. It works well.

Ready to go.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Sep 20, 2025

Anything else?

@kwankyu kwankyu force-pushed the p/fix-doc-location branch from f7cafd3 to aed9e8b Compare December 2, 2025 02:09
@kwankyu
Copy link
Collaborator Author

kwankyu commented Dec 2, 2025

Now the built doc is not copied but links to the doc are installed in appropriate places. This works well with the sage-distro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tobiasdiez tobiasdiez tobiasdiez left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c: documentation s: needs review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kwankyu @antonio-rojas @tobiasdiez